Netlify deploy preview

https://deploy-preview-37430--material-ui.netlify.app/

Bundle size report

No bundle size changes

Generated by 🚫 dangerJS against 374f644

The failing test is this Tooltip followCursor test, which is timing out. Investigating.

Update: The issue is coming from @sinonjs/[email protected]. The test passes with @sinonjs/[email protected] and @sinonjs/[email protected], which is the version before 1.2.0. I think this change broke it but don't know why yet.

cc: @mui/core looking at the change I'm suspicious about, does something catch your attention as the reason why it broke the Tooltip test?

looking at the change I'm suspicious about, does something catch your attention as the reason why it broke the Tooltip test?

I couldn't find a clear explanation or solution for the issue. However, based on my investigation, I observed that the test passes when run on the browser environment, but fails when run on JSDom, which is a testing environment that simulates Node.js.

Also I see the stack trace pointing to the following lines in Node.js timers module:

      at listOnTimeout (internal/timers.js:557:17)
      at processTimers (internal/timers.js:500:7)

They added mocks to the timers module in in sinonjs/fake-timers#467 (v10.2.0).

Since the same problem is also blocking a pull request in the MUI X repository (mui/mui-x#9063), it might be worth opening an issue in the fake-timers repository (https://github.com/sinonjs/fake-timers/issues) to seek further assistance and resolution for this issue.

Sinon maintainer here. Since you are not using fake timers directly I followed the rendering utils suspects in
https://github.com/mui/material-ui/blob/master/packages/mui-material/src/Tooltip/Tooltip.test.js#L4.

In https://github.com/mui/material-ui/blob/master/test/utils/createRenderer.tsx#L18 the fake timers are setup, nothing scary that I can see.

I am guessing this is basically a case of JSOM's timers and Node's timers fighting it out (window.setTimeout vs global.setTimeout). You can see what I think is a similar issue here: jestjs/jest#625

Figuring out exactly which line needs to be changed, if it is the setup (changing the target for the stubbing from the implicit global to window), if it is the client code that should be calling window.setTimeout or the test util that should somehow be changed I am not immediately sure.

We did not expect this to break uses of fake-timers, so we can see if we should retract (deprecate) this minor version and re-release it as a new major version to avoid this. Ditto for sinon.

I don't have the possibility to debug the details right now, but could you try to pinpoint which sets of timers (global.setTimeout vs window.setTimeout) is being stubbed and which should have been stubbed, etc.? And maybe see if choosing an explicit target to fake the clock for could be a thing? Alternatively, add global.setTimeout = window.setTimeout or some variations thereof. Would be nice to know if it can be worked around (which we could document) or if it needs a new flag (nodeBuiltins: false).

This should pass if changing to use Sinon 15.1.2 (which targets the older version). Still, would like to know if the above section could point out what is wrong.

While I could reproduce locally by running yarn test:unit, I was not able to run the single test suite, as it would fail all tests in that file when run separately with the same error:

  MUI: The `anchorEl` prop provided to the component is invalid.
The anchor element should be part of the document layout.
Make sure the element is present in the document or that it's not display none.

Stack:
    at /home/carlerik/code/material-ui/packages/mui-base/src/Popper/Popper.tsx:155:19
    at commitHookEffectListMount (/home/carlerik/code/material-ui/node_modules/react-dom/cjs/react-dom.development.js:23150:26)

No idea why. Used

$ yarn mocha --config .mocharc.js /home/carlerik/code/material-ui/packages/mui-material/src/Tooltip/Tooltip.test.js
  .!!.!.!.!..!.!.!.!..!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.......!.!.!.!.!.!.!.!.!.!,,,..!.!.......!.!...!.!...!..!.!.!.!.!........!...!.!.!.!.!.!....!.!.!..^C

@fatso83 thanks for taking a look into it!

I think updating to sinon 15.1.2 is not working as that version uses fake-timers version ^10.1.0, so it's being bumped to 10.2.0, the latest minor, which has the issue. Either releasing a 10.3.0 version that reverts the timers mock or marking fake-timers version as ~10.1.0 might work, whatever makes more sense.

I tried all the variations of global.setTimeout = window.setTimeout that I could think of but nothing worked. If there's something else I can try to help debugging please let me know. Maybe it's another method that's causing the issue.

To run the test locally, you can use yarn test:unit --grep Tooltip. That will run only the Tooltip tests. You can also use it.only on the test to run only the failing case:

describe('prop: followCursor', () => {
    it.only('should use the position of the mouse', async function test() {
      // ...

Hopefully re-releasing 10.1.0 as 10.3.0 will fix this for you guys. I have no idea why the changes did not work for you guys, though, but this will probably give you some more time. https://www.npmjs.com/package/@sinonjs/fake-timers?activeTab=versions

The new failure is related to Webpack 5 adjustments we just merged in Sinon 15.2. You probably need to change your polyfills setup.

Similar: microsoft/PowerBI-visuals-tools#365

Edited/Blocked Notification

Renovate will not automatically rebase this PR, because it does not recognize the last commit author and assumes somebody else may have edited the PR.

You can manually request rebase by checking the rebase/retry box above.

⚠ Warning: custom changes will be lost.

Hey @michaldudak, may I ask for your review here? required a slight change in the regression's test webpack config: https://github.com/mui/material-ui/pull/37430/files#diff-c6283f5fa0c0e1715f4d0fdac062964728ebcd04b2868febd6efa5b1b4017600R28

All seems to be good with regression tests - good job!

Thanks @fatso83 for taking a look into it and helping us update! 🚀

No worries, glad to help. that still leaves the mystery of what originally breaks your tests. As soon as we release Sinon 16 with fake-timers 11 this will resurface. Hopefully someone is able to create a trimmed-down minimal testcase by then.